bip-0375: assign k in output index order, fix labeled test vectors - #2256

@fametrano Thank you for your proposal. Can you summarize the PR description more concisely in your own words, please.

@jonatack fair request, in my own words:

BIP375's text says to sort the silent payment codes lexicographically to assign k; instead, its validator and its vectors assign k by output index. This PR changes the text to match the vectors, #2207 changes the vectors to match the text; either removes the contradiction. I prefer index order because it needs no definition of how a "code" is encoded and compared, and the outputs cannot be reordered once the scripts are set anyway (L185).

To reproduce in a minute, apply this to bip-0375/validator/validate_psbt.py and run python3 test_runner.py in bip-0375/:

@@ -317,11 +317,13 @@
     scan_key_k_values = {}

     # Validate each SP output
-    for output_idx, output_map in enumerate(psbt.o):
-        if PSBT_OUT_SP_V0_INFO not in output_map:
-            continue  # Skip non-SP outputs
-
-        sp_info = output_map[PSBT_OUT_SP_V0_INFO]
+    sp_outputs = [
+        (output_map[PSBT_OUT_SP_V0_INFO], output_idx, output_map)
+        for output_idx, output_map in enumerate(psbt.o)
+        if PSBT_OUT_SP_V0_INFO in output_map
+    ]
+    sp_outputs.sort(key=lambda entry: (entry[0], entry[1]))
+    for sp_info, output_idx, output_map in sp_outputs:
         scan_pubkey_bytes = sp_info[:33]
         spend_pubkey_bytes = sp_info[33:]

Today, at master 09e2103: unchanged, 42 passed, 0 failed; with the sort, 41 passed, 1 failed, the failure being "two sp outputs - output 0 uses label=3 / output 1 uses label=1", whose spend keys are in descending order. (My description above says valid[8] for that vector; it is index 9 in the file.)

The PR also rewords two vector descriptions that no longer said what they test once the text is index order ("not sorted lexicographically by spend key" names a rule the text no longer states): both are reworded, in the file and in the BIP's table, and the validator's counter comment now says which order it follows. No vector bytes change.

On #2207. If the owners prefer this direction, #2207's sort and its ordering-driven vector regeneration become unnecessary, but its other half, the labeled spend key in PSBT_OUT_SP_V0_INFO, is independent and still needed: I would suggest reducing #2207 to that half rather than closing it, and I am glad to help rebase it on this. If the owners prefer #2207's direction, this one should be closed instead.

@fametrano #2207 was opened to match the vectors and validation with the BIP text.

@macgyver13 right, #2207 and this PR fix the same contradiction in opposite directions: #2207 edits the vectors and validator to match the prose (lexicographic), this PR edits the prose to match the vectors and validator (output-index order).

Measured against master (09e2103): the validator as published assigns k by output index and passes all 42 vectors. Applying #2207's lexicographic sort instead gives 41/42 — the "two sp outputs, output 0 label=3 / output 1 label=1" vector, published as valid, no longer produces its own PSBT_OUT_SCRIPTs, because its two spend keys are in descending order.

So the vectors and the reference validator already agree on index order; only the prose dissents. That's why I proposed changing the sentence rather than the vectors. If the editors prefer #2207's direction, that vector's expected scripts have to change too. Happy to go whichever way they decide.

nACK

Prefer #2207. Correcting implementations to match BIP text should be the default.

Also e4ba7f8 includes pycache files

Thanks for your input @macgyver13. @fametrano: I think it’s up to the owners of the BIP rather than the Editors whether they want to adjust the text of the BIP or the implementation.
cc: @andrewtoth, @achow101, @josibake

Thanks @macgyver13 — good catch. The stray __pycache__ files are removed from the branch; nothing under bip-0375/deps or the validator carries bytecode any more. The direction itself is for the BIP owners to weigh, as @murchandamus notes.

Rebased onto master after #2286, merged by jonatack (only conflict was the changelog, now 0.1.3).

@andrewtoth @achow101 @josibake, this and #2207 still need your call on which way to fix the k ordering.

Perhaps the best path here might be a mix of the two pulls:

• The rule from this PR bip-0375: assign k in output index order, fix labeled test vectors #2256: k in output-index order per scan key
• The field fix from BIP-375: correct labeled test vectors to include labeled-spend key in PSBT_OUT_SP_V0_INFO #2207: PSBT_OUT_SP_V0_INFO carries the labeled spend key

Edit: I see a similar suggestion in #2256 (comment): "the labeled spend key in PSBT_OUT_SP_V0_INFO, is independent and still needed: I would suggest reducing #2207 to that half rather than closing it, and I am glad to help rebase it on this." Yes, agree.

Thanks @jonatack, done: this now has the ordering rule from here and the PSBT_OUT_SP_V0_INFO fix from #2207, with @macgyver13 as co-author on the commit. I had suggested splitting that fix out of #2207 (Sep 9), since it is independent of the ordering. @macgyver13, please check that I carried it over correctly.

The BIP-375 test vectors that assign k across multiple outputs sharing a scan key, and the reference validator's k assignment, are wrong, and I'm the one who wrote them. I applied the BIP-352 output creation procedure over the outputs in index order and missed BIP-375's added requirement to sort the codes lexicographically before assigning k. The vectors and the validator agree with each other, but that is one error, not two independent readings of the spec.

@jvgelder was the first to point out that the vectors didn't match the text, while implementing Caravan. That's why I opened #2207.

Implementations that follow the text as written:

• Sparrow 2.5.5, released (drongo 59f43e3)
• SeedSigner SeedSigner/seedsigner#949, draft (via embit)
• Caravan caravan-bitcoin/caravan#496, open
• Coldcard Coldcard/firmware#587, open. This one is my own implementation and hasn't been merged.

All of them sort by the 33-byte spend key in PSBT_OUT_SP_V0_INFO, then by output index. Ordering doesn't affect receivers, so this is purely about signer interoperability.

@fametrano, are you aware of any other implementations besides btclib that assign k in output index order? It would help to have the full picture on both sides.

#2316 fixes the P2WPKH programs and signatures in bip375_test_vectors.json, and it conflicts with this PR in that file. This PR's new and edited vectors with P2WPKH inputs carry the same wrong program (SHA256 instead of HASH160). Whichever of the two lands second will be rebased onto the other, with those inputs repaired the same way. The resolved file is ready and checked for both orders.

@macgyver13, thanks for the clear account.

No, I'm not aware of any other. btclib followed the vectors, and it will follow whichever ordering the BIP settles on.

That said, the BIP is still a Draft, and I don't think early implementations should settle this decision.